Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose Run instance on Task #1018

Closed

Conversation

olivier-thatch
Copy link

Expose Run instance on Task.

This is basically the same as #924, so all credit goes to @shalvah. I'm opening a new PR since the old one was closed and I'm hoping a new PR will be more visible to the maintainers (but also happy to close this PR in favor of the old one).

We've been using this in our codebase via the following monkey patch:

require "maintenance_tasks"

module MaintenanceTasks
  class Run < ApplicationRecord
    module SetTaskRunPatch
      def task
        super.tap do |task|
          task.run ||= self
        end
      end
    end

    prepend SetTaskRunPatch
  end

  class Task
    attr_accessor :run
  end
end

This enables us to do some interesting stuff like exposing a logger to tasks and storing the log output on the Run instances, or adding the run ID to our error contexts, etc.

@olivier-thatch
Copy link
Author

@etiennebarrie Sorry for the ping but would love to get some feedback on this PR :)

@etiennebarrie
Copy link
Member

Our stance has been not to expose Run itself, so that it stays an internal concern, and solving the actual issues directly. For example:

adding the run ID to our error contexts

Since we added #941, we should probably also add the id to the error task context:

task_context = {
task_name: @run.task_name,
started_at: @run.started_at,
ended_at: @run.ended_at,
}

@github-actions github-actions bot added the stale label Aug 12, 2024
@github-actions github-actions bot closed this Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants